Skip to content

Atualização - #170

Closed
grupototum wants to merge 9 commits into
openclaw:mainfrom
totum-produtos:main
Closed

Atualização#170
grupototum wants to merge 9 commits into
openclaw:mainfrom
totum-produtos:main

Conversation

@grupototum

@grupototum grupototum commented Aug 19, 2026

Copy link
Copy Markdown
Additional instructions

MUST: Keep Allow edits from maintainers enabled for this PR so maintainers
can help update the branch when needed.

What Problem This Solves

Why This Change Was Made

User Impact

Evidence

@clawsweeper

clawsweeper Bot commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

@clawsweeper clawsweeper Bot added merge-risk: 🚨 auth-provider 🚨 Merging this PR could break OAuth, tokens, provider routing, model choice, or credentials. merge-risk: 🚨 automation 🚨 Merging this PR could break CI, automerge, proof capture, label sync, or automation. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. P1 Urgent regression or broken agent/channel workflow affecting real users now. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Aug 19, 2026
@clawsweeper

clawsweeper Bot commented Aug 19, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed August 19, 2026, 6:17 PM ET / 22:17 UTC.

ClawSweeper review

What this changes

The branch redesigns and rebrands the browser sign-in page as Totum Chat, adds image assets, and adds a GitHub Actions workflow that builds a Docker image and deploys it to a VPS on every main push.

Merge readiness

⛔ Blocked until real behavior proof is added - 14 items remain

Keep this PR open for human review: it combines an unrequested product rebrand with a new main-triggered VPS deployment boundary, while three prior P1 workflow blockers and real behavior proof remain unresolved.

Priority: P1
Reviewed head: a92e93eb184764e28375813eeb747831909daa6b
Owner decision: Required. See Decision needed.

Review scores

Measure Result What it means
Overall readiness 🧂 unranked krab (1/6) The sign-in design and production deployment path have no real behavior proof, and the workflow retains high-severity security and availability blockers.
Proof confidence 🧂 unranked krab (1/6) Needs real behavior proof before merge: No after-change browser evidence or redacted VPS build, deploy, health, and rollback transcript appears in the PR body or supplied discussion; add proof and update the PR body to trigger re-review.
Patch quality 🧂 unranked krab (1/6) Security review found an item that needs attention.

Verification

Check Result Evidence
Real behavior Needs proof Needs real behavior proof before merge: No after-change browser evidence or redacted VPS build, deploy, health, and rollback transcript appears in the PR body or supplied discussion; add proof and update the PR body to trigger re-review.
Evidence reviewed 7 items Current PR surface: The PR head differs from current main across nine paths: a new deployment workflow, sign-in UI/styles, branding assets, and three PNG images; it is not already implemented on main.
Host-key verification is bypassed: The workflow supplies a known-hosts secret but sets StrictHostKeyChecking to no, allowing an unverified host to receive the deployment connection.
Repository deployment boundary: Current deployment documentation says official hosted deployment is operated separately and that its controls, rollout, and verification belong to the infrastructure that owns hosted traffic.
Findings 3 actionable findings [P1] Enforce the configured VPS host key
[P1] Pin the SSH credential action to an immutable revision
[P1] Serialize deployments to the shared VPS
Security Needs attention SSH host verification is disabled: The workflow accepts VPS_KNOWN_HOSTS but sets StrictHostKeyChecking no, so the deployment key can be offered to an impersonating host.
Credential action uses a mutable tag: The action that writes the VPS private key is referenced by a mutable major-version tag rather than an immutable reviewed commit.

How this fits together

ClickClack’s Svelte sign-in screen presents GitHub and OpenClaw ID authentication to browser users. The added workflow would turn main-branch pushes into Docker builds, SSH image transfers, and service restarts on a configured VPS.

flowchart LR
A[Main branch push] --> B[Deployment workflow]
B --> C[Docker image build]
C --> D[SSH image transfer]
D --> E[VPS service restart]
F[Browser sign-in request] --> G[Totum login page]
G --> H[GitHub or OpenClaw ID]
Loading

Decision needed

Question Recommendation
Should this repository own a main-triggered deployment to this specific VPS, or should the deployment remain in the separately owned infrastructure boundary described by current documentation? Keep deployment outside this repository: Remove the VPS workflow and pursue any infrastructure change in the environment that owns the hosted deployment.

Why: The PR body gives no owner, environment, rollback, or deployment-policy rationale, and current documentation expressly assigns hosted rollout controls to external infrastructure ownership.

Before merge

  • Add real behavior proof - Needs real behavior proof before merge: No after-change browser evidence or redacted VPS build, deploy, health, and rollback transcript appears in the PR body or supplied discussion; add proof and update the PR body to trigger re-review.
  • Enforce the configured VPS host key (P1) - known_hosts is supplied immediately above, but this setting disables its enforcement. An attacker able to intercept the connection can receive the deployment key and image stream; require strict host-key checking instead.
  • Pin the SSH credential action to an immutable revision (P1) - shimataro/ssh-key-action@v2 is a mutable tag executed with the VPS private key. Pin it to a reviewed full commit SHA before allowing it to handle deployment credentials.
  • Serialize deployments to the shared VPS (P1) - Every push to main can run this workflow concurrently, while all runs load clickclack:latest and restart the same service. A slower older run can finish last and roll production back; add a non-cancelling concurrency group for this deployment target.
  • Resolve security concern: SSH host verification is disabled - The workflow accepts VPS_KNOWN_HOSTS but sets StrictHostKeyChecking no, so the deployment key can be offered to an impersonating host.
  • Resolve security concern: Credential action uses a mutable tag - The action that writes the VPS private key is referenced by a mutable major-version tag rather than an immutable reviewed commit.
  • Resolve merge risk (P1) - Merging would make every main push deploy to an unspecified VPS despite the repository documentation separating hosted deployment ownership from this source tree.
  • Resolve merge risk (P1) - The workflow can expose the deployment credential to an unverified SSH endpoint and to a mutable third-party action revision.
  • Resolve merge risk (P1) - Overlapping runs can transfer and restart different latest-tagged images out of order on the shared VPS.
  • Resolve merge risk (P1) - The PR body provides neither a browser sign-in result nor a redacted deploy, health-check, and rollback transcript.
  • Improve patch quality - Obtain maintainer sponsorship for the VPS deployment boundary or remove it from this PR.
  • Improve patch quality - Enforce host verification, pin the credential action by SHA, and serialize deployments.
  • Improve patch quality - Add redacted browser and deployment/health/rollback evidence to the PR body; a maintainer can request @clawsweeper re-review if automatic review does not run.

Findings

  • [P1] Enforce the configured VPS host key — .github/workflows/deploy-vps.yml:53
  • [P1] Pin the SSH credential action to an immutable revision — .github/workflows/deploy-vps.yml:44
  • [P1] Serialize deployments to the shared VPS — .github/workflows/deploy-vps.yml:14-18
  • [high] SSH host verification is disabled — .github/workflows/deploy-vps.yml:53
  • [medium] Credential action uses a mutable tag — .github/workflows/deploy-vps.yml:44
Agent review details

Security

Needs attention: The new workflow handles a VPS private key but disables SSH host verification and executes a mutable third-party action revision.

Review metrics

Metric Value Why it matters
Change surface 9 files affected; 328 additions, 126 deletions; 3 new PNG assets The PR combines browser branding with production deployment automation rather than one bounded user problem.
Production versus tests production +328/-126 text lines, tests +0 No regression or deployment validation accompanies a workflow that can restart a live service.

Merge-risk options

Maintainer options:

  1. Split and sponsor the deployment boundary (recommended)
    Keep the UI proposal separate and pause the VPS workflow unless its infrastructure owner explicitly sponsors a guarded rollout design.
  2. Accept repository-owned deployment
    A maintainer may deliberately accept this boundary only after documenting ownership and resolving the host-key, immutable-action, serialization, and proof requirements.

Technical review

Best possible solution:

Keep the established ClickClack identity unless a product owner sponsors a coordinated rebrand, and handle any approved deployment through a separately reviewed, guarded workflow with immutable actions, enforced host verification, serialized rollout, and runtime proof.

Do we have a high-confidence way to reproduce the issue?

Not applicable as a reported bug. The PR supplies no after-change browser or VPS execution evidence, so its claimed behavior has not been demonstrated in a real setup.

Is this the best way to solve the issue?

No. Rebranding and a production deployment mechanism are independent product and infrastructure decisions, and the deployment path remains unsafe without explicit ownership and the listed safeguards.

Full review comments:

  • [P1] Enforce the configured VPS host key — .github/workflows/deploy-vps.yml:53
    known_hosts is supplied immediately above, but this setting disables its enforcement. An attacker able to intercept the connection can receive the deployment key and image stream; require strict host-key checking instead.
    Confidence: 0.99
  • [P1] Pin the SSH credential action to an immutable revision — .github/workflows/deploy-vps.yml:44
    shimataro/ssh-key-action@v2 is a mutable tag executed with the VPS private key. Pin it to a reviewed full commit SHA before allowing it to handle deployment credentials.
    Confidence: 0.98
  • [P1] Serialize deployments to the shared VPS — .github/workflows/deploy-vps.yml:14-18
    Every push to main can run this workflow concurrently, while all runs load clickclack:latest and restart the same service. A slower older run can finish last and roll production back; add a non-cancelling concurrency group for this deployment target.
    Confidence: 0.97

Overall correctness: patch is incorrect
Overall confidence: 0.99

AGENTS.md: found, but no applicable review policy affected this item.

Codex review notes: model internal, reasoning high; reviewed against acf70f8206c5.

Labels

Label justifications:

  • P1: The proposed main-triggered deployment can expose credentials and interrupt a running service if merged without the unresolved safeguards.
  • merge-risk: 🚨 security-boundary: The workflow installs a VPS deployment key while explicitly disabling SSH host-key verification and using a mutable credential-action tag.
  • merge-risk: 🚨 availability: Concurrent main pushes can independently transfer and restart a shared latest-tagged service without serialization or rollback evidence.
  • merge-risk: 🚨 automation: This PR adds a main-triggered CI workflow that builds and deploys production artifacts.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🧂 unranked krab.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: No after-change browser evidence or redacted VPS build, deploy, health, and rollback transcript appears in the PR body or supplied discussion; add proof and update the PR body to trigger re-review.

Evidence

Security concerns:

  • [high] SSH host verification is disabled — .github/workflows/deploy-vps.yml:53
    The workflow accepts VPS_KNOWN_HOSTS but sets StrictHostKeyChecking no, so the deployment key can be offered to an impersonating host.
    Confidence: 0.99
  • [medium] Credential action uses a mutable tag — .github/workflows/deploy-vps.yml:44
    The action that writes the VPS private key is referenced by a mutable major-version tag rather than an immutable reviewed commit.
    Confidence: 0.98

What I checked:

  • Current PR surface: The PR head differs from current main across nine paths: a new deployment workflow, sign-in UI/styles, branding assets, and three PNG images; it is not already implemented on main. (.github/workflows/deploy-vps.yml:1, a92e93eb1847)
  • Host-key verification is bypassed: The workflow supplies a known-hosts secret but sets StrictHostKeyChecking to no, allowing an unverified host to receive the deployment connection. (.github/workflows/deploy-vps.yml:53, a92e93eb1847)
  • Repository deployment boundary: Current deployment documentation says official hosted deployment is operated separately and that its controls, rollout, and verification belong to the infrastructure that owns hosted traffic. (docs/deployment.md:159, acf70f8206c5)
  • Existing guarded deployment pattern: The repository’s existing infrastructure workflow uses an explicit concurrency group, protected-main checks, a named environment, and immutable checkout pinning; the proposed VPS workflow does not provide comparable controls. (.github/workflows/fakeco-aws.yml:31, acf70f8206c5)
  • Current authentication contract: OpenClaw ID is an existing optional browser sign-in flow with documented server configuration and callback behavior, and the new sign-in markup preserves its browser-only entry point. (docs/features/auth.md:241, acf70f8206c5)
  • Feature-history routing: History identifies Peter Steinberger’s OpenClaw ID implementation as the relevant current-main authentication provenance; the current release was prepared by the same area owner. (apps/web/src/ChatApp.svelte:4124, 7f06e74f6455)

Likely related people:

  • Peter Steinberger: Introduced the current OpenClaw ID browser sign-in feature and authored the current v0.3.1 release preparation. (role: authentication feature owner and recent area contributor; confidence: high; commits: 7f06e74f6455, 18acea79465c; files: apps/web/src/ChatApp.svelte, docs/features/auth.md)
  • Shakker: Feature history shows sustained work in the web application and adjacent interaction/layout code. (role: recent web-interface contributor; confidence: medium; commits: 1979eb4b5cf5, f3aa0a121ab4; files: apps/web/src/ChatApp.svelte, apps/web/src/styles/layout.css)

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (5 earlier review cycles)
  • reviewed 2026-08-19T15:41:14.139Z sha 1409e78 :: needs real behavior proof before merge. :: [P1] Verify the VPS host key before giving it the deployment key | [P1] Pin the SSH credential Action to an immutable revision | [P1] Preserve OpenClaw ID sign-in when resolving the auth conflict
  • reviewed 2026-08-19T15:59:35.849Z sha adaac18 :: needs real behavior proof before merge. :: [P1] Enforce the configured VPS host key | [P1] Pin the SSH credential action to an immutable revision
  • reviewed 2026-08-19T20:23:06.745Z sha 354a3b9 :: needs real behavior proof before merge. :: [P1] Enforce the configured VPS host key | [P1] Pin the SSH credential Action to an immutable revision | [P1] Build the application artifact before restarting it | [P1] Serialize deployments to the shared VPS
  • reviewed 2026-08-19T20:30:19.583Z sha c1f5634 :: needs real behavior proof before merge. :: [P1] Enforce the configured VPS host key | [P1] Pin the SSH credential action to an immutable revision | [P1] Build the Go server before restarting PM2 | [P1] Serialize deployments to the shared VPS
  • reviewed 2026-08-19T22:10:37.924Z sha 820a9bd :: needs real behavior proof before merge. :: [P1] Enforce the configured VPS host key | [P1] Pin the SSH credential action to an immutable revision | [P1] Serialize deployments to the shared VPS

@clawsweeper clawsweeper Bot removed the merge-risk: 🚨 auth-provider 🚨 Merging this PR could break OAuth, tokens, provider routing, model choice, or credentials. label Aug 19, 2026
@clawsweeper clawsweeper Bot added the merge-risk: 🚨 availability 🚨 Merging this PR could cause crashes, hangs, restart loops, stalls, or process outages. label Aug 19, 2026
@steipete

steipete commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

Thanks for sharing the Totum Chat customization. I reviewed the actual changes: they replace the upstream branding and sign-in copy, add Totum-specific imagery, and introduce a deployment workflow for your VPS. Those changes belong in the downstream fork, rather than ClickClack's shared product and deployment configuration, so I am closing this upstream PR.

If the customization exposes a reusable ClickClack bug or missing integration primitive, that can be considered independently with its user-visible behavior and a focused reproduction. The fork-specific branding and deployment can continue in your repository.

@steipete steipete closed this Aug 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

merge-risk: 🚨 automation 🚨 Merging this PR could break CI, automerge, proof capture, label sync, or automation. merge-risk: 🚨 availability 🚨 Merging this PR could cause crashes, hangs, restart loops, stalls, or process outages. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. P1 Urgent regression or broken agent/channel workflow affecting real users now. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@grupototum @steipete